Add cloudstack_role_permission resource by bddvlpr · Pull Request #300 · apache/cloudstack-terraform-provider

bddvlpr · 2026-07-09T08:07:04Z

Creates the cloudstack_role_permission resource for managing individual allow/deny rules on a role where previously this was not possible through Terraform. Rules target an API name or wildcard and support in-place toggling between allow and deny without replacement.

Tested on the basic CRUD operations on the simulator and an actual environment.

Copilot

Pull request overview

This PR introduces a new Terraform resource for managing individual CloudStack role permission rules (allow/deny) so role ACLs can be managed at per-API (or wildcard) granularity from Terraform.

Changes:

Adds cloudstack_role_permission resource with CRUD support, including in-place updates for allow/deny toggling.
Adds an acceptance test covering basic create + update of the permission attribute.
Adds end-user documentation and wires the new resource into the website sidebar and provider resource map.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`website/docs/r/role_permission.html.markdown`	Adds documentation for the new `cloudstack_role_permission` resource, including usage and arguments.
`website/cloudstack.erb`	Adds the resource to the docs sidebar navigation.
`cloudstack/resource_cloudstack_role_permission.go`	Implements the new role-permission Terraform resource (Create/Read/Update/Delete).
`cloudstack/resource_cloudstack_role_permission_test.go`	Adds an acceptance test for create + update behavior.
`cloudstack/provider.go`	Registers the new resource in the provider’s resource map.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+			{
+				Config: testAccCloudStackRolePermission_update,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckCloudStackRolePermissionExists("cloudstack_role_permission.foo", &rolePermission),
+					resource.TestCheckResourceAttr(
+						"cloudstack_role_permission.foo", "permission", "deny"),
+				),
+			},


bddvlpr · 2026-07-31T08:59:08Z

Updating this in a bit, I've got a few issues with the depends_on ordering. Will resort to a list instead.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

cloudstack/resource_cloudstack_role_permission.go:204

With authoritative = true, Delete removes all role permissions, including any that may have been added out-of-band after the last apply. This is a significant behavioral edge case that is not currently documented in the new resource docs; please document this explicitly (or adjust behavior to only delete permissions known to be managed by this resource, if that’s the intended contract).

	if d.Get("authoritative").(bool) {
		for _, rp := range rolePermissions {
			if err := deleteCloudStackRolePermission(cs, rp.Id); err != nil {
				return err
			}
		}
		return nil
	}

cloudstack/resource_cloudstack_role_permission.go:81

The schema uses permission as both the block name (permission { ... }) and an attribute inside the block (permission = \"allow\"|\"deny\"), which is confusing in configuration and documentation. Since this is a new resource, consider renaming the inner attribute to something unambiguous like effect, access, or action (keeping allow/deny values) to improve UX and reduce misreads.

			"permission": {
				Type:        schema.TypeList,
				Optional:    true,
				Description: "Ordered list of role permission rules. Rules are evaluated from top to bottom.",
				Elem: &schema.Resource{
					Schema: map[string]*schema.Schema{
						"id": {
							Type:        schema.TypeString,
							Computed:    true,
							Description: "ID of the role permission.",
						},
						"rule": {
							Type:        schema.TypeString,
							Required:    true,
							Description: "The API name or wildcard (e.g. 'list*') the permission applies to.",
						},
						"permission": {
							Type:         schema.TypeString,
							Required:     true,
							ValidateFunc: validation.StringInSlice([]string{"allow", "deny"}, false),
							Description:  "Whether the rule is allowed or denied. Valid options are: allow, deny.",
						},

Copilot

🟡 Not ready to approve

The acceptance test CheckDestroy implementation does not currently validate that managed role-permission rules are actually removed, so delete behavior can regress without failing tests.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (1)

cloudstack/resource_cloudstack_role_permission_test.go:274

testAccCheckCloudStackRolePermissionDestroy currently only verifies that ListRolePermissions doesn’t error; it never asserts that managed permission rules were actually removed during destroy. This means the acceptance tests would still pass even if the resource Delete implementation were a no-op.

func testAccCheckCloudStackRolePermissionDestroy(s *terraform.State) error {
	cs := testAccProvider.Meta().(*cloudstack.CloudStackClient)

	for _, rs := range s.RootModule().Resources {
		if rs.Type != "cloudstack_role_permission" {

Files reviewed: 5/5 changed files
Comments generated: 0 new
Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

…tive management

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (2)

cloudstack/resource_cloudstack_role_permission.go:115

resourceCloudStackRolePermissionRead falls back to using d.Id() when role_id is empty (e.g., after import), but it never writes role_id back into state and also doesn’t normalize the resource ID. Because role_id is ForceNew, an imported resource can end up planning a replacement just to populate role_id. Set role_id and the resource ID when role_id is missing so state remains stable.

	roleID := d.Get("role_id").(string)
	if roleID == "" {
		roleID = d.Id()
	}

cloudstack/resource_cloudstack_role_permission.go:155

With authoritative=true, out-of-band role permissions that exist in CloudStack but aren’t declared in configuration are not represented in state during Read. That means Terraform won’t detect drift and won’t plan an Update to delete the undeclared permissions (despite the authoritative contract in the docs). Include any undeclared remote permissions in state when authoritative=true so the next plan/apply can remove them.

	if err := d.Set("permission", readPermissions); err != nil {
		return fmt.Errorf("Error setting Role Permissions: %s", err)
	}

kiranchavala

LGTM , Tested manually

STEP 1: basic role creation

resource "cloudstack_role_permission" "foo" {
  role_id = cloudstack_role.foo.id

  permission {
    rule        = "listVirtualMachines"
    permission  = "allow"
    description = "terraform test role permission"
  }
}

terraform apply


Terraform used the selected providers to generate the following execution plan. Resource actions are indicated
with the following symbols:
  + create

Terraform will perform the following actions:

  # cloudstack_role.foo will be created
  + resource "cloudstack_role" "foo" {
      + description = (known after apply)
      + id          = (known after apply)
      + is_public   = true
      + name        = "pr300-test-role"
      + type        = "User"
    }

  # cloudstack_role_permission.foo will be created
  + resource "cloudstack_role_permission" "foo" {
      + authoritative = false
      + id            = (known after apply)
      + role_id       = (known after apply)

      + permission {
          + description = "terraform test role permission"
          + id          = (known after apply)
          + permission  = "allow"
          + rule        = "listVirtualMachines"
        }
    }

Plan: 2 to add, 0 to change, 0 to destroy.
╷
│ Warning: Value for undeclared variable
│ 
│ The root module does not declare a variable named "physical_network_id" but a value was found in file
│ "terraform.tfvars.json". If you meant to use this value, add a "variable" block to the configuration.
│ 
│ To silence these warnings, use TF_VAR_... environment variables to provide certain "global" settings to all
│ configurations in your organization. To reduce the verbosity of these warnings, use the -compact-warnings
│ option.
╵

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

cloudstack_role.foo: Creating...
cloudstack_role.foo: Creation complete after 1s [id=0c8db044-56e0-4a0f-8a2f-5dbe57232cf2]
cloudstack_role_permission.foo: Creating...
cloudstack_role_permission.foo: Creation complete after 0s [id=0c8db044-56e0-4a0f-8a2f-5dbe57232cf2]

Apply complete! Resources: 2 added, 0 changed, 0 destroyed.

Step2 : Update the role permission

resource "cloudstack_role_permission" "foo" {
  role_id = cloudstack_role.foo.id

  permission {
    rule        = "listVirtualMachines"
    permission  = "deny"
    description = "terraform test role permission"
  }
}

terraform apply 
cloudstack_role.foo: Refreshing state... [id=0c8db044-56e0-4a0f-8a2f-5dbe57232cf2]
cloudstack_role_permission.foo: Refreshing state... [id=0c8db044-56e0-4a0f-8a2f-5dbe57232cf2]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated
with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # cloudstack_role_permission.foo will be updated in-place
  ~ resource "cloudstack_role_permission" "foo" {
        id            = "0c8db044-56e0-4a0f-8a2f-5dbe57232cf2"
        # (2 unchanged attributes hidden)

      ~ permission {
            id          = "7d007208-191d-4d89-866e-3181f50891dc"
          ~ permission  = "allow" -> "deny"
            # (2 unchanged attributes hidden)
        }
    }

Plan: 0 to add, 1 to change, 0 to destroy.
╷
│ Warning: Value for undeclared variable
│ 
│ The root module does not declare a variable named "physical_network_id" but a value was found in file
│ "terraform.tfvars.json". If you meant to use this value, add a "variable" block to the configuration.
│ 
│ To silence these warnings, use TF_VAR_... environment variables to provide certain "global" settings to all
│ configurations in your organization. To reduce the verbosity of these warnings, use the -compact-warnings
│ option.
╵

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

cloudstack_role_permission.foo: Modifying... [id=0c8db044-56e0-4a0f-8a2f-5dbe57232cf2]
cloudstack_role_permission.foo: Modifications complete after 1s [id=0c8db044-56e0-4a0f-8a2f-5dbe57232cf2]

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.

Step3 ordering

resource "cloudstack_role_permission" "foo" {
  role_id = cloudstack_role.foo.id

  permission {
    rule       = "listZones"
    permission = "allow"
  }

  permission {
    rule       = "*"
    permission = "deny"
  }
}

terraform apply 
cloudstack_role.foo: Refreshing state... [id=0c8db044-56e0-4a0f-8a2f-5dbe57232cf2]
cloudstack_role_permission.foo: Refreshing state... [id=0c8db044-56e0-4a0f-8a2f-5dbe57232cf2]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated
with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # cloudstack_role_permission.foo will be updated in-place
  ~ resource "cloudstack_role_permission" "foo" {
        id            = "0c8db044-56e0-4a0f-8a2f-5dbe57232cf2"
        # (2 unchanged attributes hidden)

      ~ permission {
          - description = "terraform test role permission" -> null
            id          = "7d007208-191d-4d89-866e-3181f50891dc"
          ~ permission  = "deny" -> "allow"
          ~ rule        = "listVirtualMachines" -> "listZones"
        }
      + permission {
          + permission = "deny"
          + rule       = "*"
        }
    }

Plan: 0 to add, 1 to change, 0 to destroy.
╷
│ Warning: Value for undeclared variable
│ 
│ The root module does not declare a variable named "physical_network_id" but a value was found in file
│ "terraform.tfvars.json". If you meant to use this value, add a "variable" block to the configuration.
│ 
│ To silence these warnings, use TF_VAR_... environment variables to provide certain "global" settings to all
│ configurations in your organization. To reduce the verbosity of these warnings, use the -compact-warnings
│ option.
╵

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

cloudstack_role_permission.foo: Modifying... [id=0c8db044-56e0-4a0f-8a2f-5dbe57232cf2]
cloudstack_role_permission.foo: Modifications complete after 1s [id=0c8db044-56e0-4a0f-8a2f-5dbe57232cf2]

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.

step 4: Test the authoritative parameter


resource "cloudstack_role_permission" "foo" {
  role_id       = cloudstack_role.foo.id
  authoritative = true

  permission {
    rule       = "listZones"
    permission = "allow"
  }
}

terraform apply

cloudstack_role.foo: Refreshing state... [id=0c8db044-56e0-4a0f-8a2f-5dbe57232cf2]
cloudstack_role_permission.foo: Refreshing state... [id=0c8db044-56e0-4a0f-8a2f-5dbe57232cf2]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated
with the following symbols:
 ~ update in-place

Terraform will perform the following actions:

 # cloudstack_role_permission.foo will be updated in-place
 ~ resource "cloudstack_role_permission" "foo" {
     ~ authoritative = false -> true
       id            = "0c8db044-56e0-4a0f-8a2f-5dbe57232cf2"
       # (1 unchanged attribute hidden)

     ~ permission {
           id          = "f805c711-06bc-4439-ac99-74cd45bff094"
         ~ permission  = "deny" -> "allow"
         ~ rule        = "*" -> "listZones"
           # (1 unchanged attribute hidden)
       }
   }

Plan: 0 to add, 1 to change, 0 to destroy.
╷
│ Warning: Value for undeclared variable
│ 
│ The root module does not declare a variable named "physical_network_id" but a value was found in file
│ "terraform.tfvars.json". If you meant to use this value, add a "variable" block to the configuration.
│ 
│ To silence these warnings, use TF_VAR_... environment variables to provide certain "global" settings to all
│ configurations in your organization. To reduce the verbosity of these warnings, use the -compact-warnings
│ option.
╵

Do you want to perform these actions?
 Terraform will perform the actions described above.
 Only 'yes' will be accepted to approve.

 Enter a value: yes

cloudstack_role_permission.foo: Modifying... [id=0c8db044-56e0-4a0f-8a2f-5dbe57232cf2]
cloudstack_role_permission.foo: Modifications complete after 0s [id=0c8db044-56e0-4a0f-8a2f-5dbe57232cf2]

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.

sureshanaparti requested review from vishesh92 and a lite review from Copilot July 10, 2026 08:22

sureshanaparti added needs-review needs-testing labels Jul 10, 2026

sureshanaparti added this to the v0.7.0 milestone Jul 10, 2026

Copilot started reviewing on behalf of sureshanaparti July 10, 2026 08:30 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

bddvlpr marked this pull request as draft July 31, 2026 08:59

bddvlpr force-pushed the feat/role-permission branch from d6abfc8 to c401de6 Compare July 31, 2026 09:00

Copilot AI review requested due to automatic review settings July 31, 2026 09:00

Copilot AI reviewed Jul 31, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 31, 2026 09:17

Copilot started reviewing on behalf of bddvlpr July 31, 2026 09:18 View session

Copilot started reviewing on behalf of bddvlpr July 31, 2026 09:24 View session

Copilot AI reviewed Jul 31, 2026

View reviewed changes

bddvlpr marked this pull request as ready for review August 9, 2026 13:11

bddvlpr added 3 commits August 10, 2026 14:03

Add cloudstack_role_permission resource

04268b0

Assert role permission update is in-place

5adbd24

Fix ordering of permissions by using a list with (optionally) authora…

65a0359

…tive management

bddvlpr force-pushed the feat/role-permission branch from fe2fa94 to 65a0359 Compare August 10, 2026 12:03

Copilot AI review requested due to automatic review settings August 10, 2026 12:03

Copilot started reviewing on behalf of bddvlpr August 10, 2026 12:04 View session

Copilot AI reviewed Aug 10, 2026

View reviewed changes

sureshanaparti approved these changes Aug 14, 2026

View reviewed changes

sureshanaparti added status:ready-for-testing and removed needs-review labels Aug 14, 2026

sureshanaparti assigned kiranchavala Aug 14, 2026

sureshanaparti requested a review from kiranchavala August 14, 2026 08:23

kiranchavala approved these changes Aug 14, 2026

View reviewed changes

kiranchavala removed needs-testing status:ready-for-testing labels Aug 14, 2026

Navigation Menu

Uh oh!

Add cloudstack_role_permission resource - #300

Add cloudstack_role_permission resource#300
bddvlpr wants to merge 3 commits into
apache:mainfrom
bddvlpr:feat/role-permission

bddvlpr commented Jul 9, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

bddvlpr commented Jul 31, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

kiranchavala left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

bddvlpr commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

bddvlpr commented Jul 31, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Not ready to approve

Suppressed comments (1)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

kiranchavala left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bddvlpr commented Jul 9, 2026 •

edited

Loading